The "path" field of a "struct repo" (a custom http-push struct, not to
be confused with "struct repository) is a pointer into a const argv
string, and is never written to.

The compiler does not traditionally complain about assigning from a
const pointer because it happens via strchr(). But with some C23 libc
versions (notably recent glibc), it has started to do so. Let's mark the
field as const to silence the warnings.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

We implicitly drop the const from our "key" variable when we do:

  char *p = strchr(key, ' ');

which causes compilation with some C23 versions of libc (notably recent
glibc) to complain.

We need "p" to remain writable, since we assign NUL over the space we
found. We can solve this by also making "key" writable. This works
because it comes from a strbuf, which is itself a writable string.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

When we do:

  char *cp = strchr(argv[i], '=');

it implicitly removes the constness from argv[i]. We need "cp" to remain
writable (since we overwrite it with a NUL). In theory we should be able
to drop the const from argv[i], because it is a sub-pointer into our
duplicated pager_env variable.

But we get it from split_cmdline(), which uses the traditional "const
char **" type for argv. This is overly limiting, but changing it would
be awkward for all the other callers of split_cmdline().

Let's do an explicit cast with a note about why it is OK. This is enough
to silence compiler warnings about the implicit const problems.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

We do this:

  char *equals = strchr(*e, '=');

which implicitly removes the constness from "*e" and cause the compiler
to complain. We never write to "equals", but later assign it to a
string_list util field, which is defined as non-const "void *".

We have to cast somewhere, but doing so at the assignment to util is the
least-bad place, since that is the source of the confusion. Sadly we are
still open to accidentally writing to the string via the util pointer,
but that is the cost of using void pointers, which lose all type
information.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

The find_last_dir_sep() function is implemented as an inline function
which takes in a "const char *" and returns a "char *" via strrchr().
That means that just like strrchr(), it quietly removes the const from
our pointer, which could lead to accidentally writing to the resulting
string.

But C23 versions of libc (including recent glibc) annotate strrchr()
such that the compiler can detect when const is implicitly lost, and it
now complains about the call in this inline function.

We can't just switch the return type of the function to "const char *",
though. Some callers really do want a non-const string to be returned
(and are OK because they are feeding a non-const string into the
function).

The most general solution is for us to annotate find_last_dir_sep() in
the same way that is done for strrchr(). But doing so relies on using
C23 generics, which we do not otherwise require.

Since this inline function is wrapping a single call to strrchr(), we
can take a shortcut. If we implement it as a macro, then the original
type information is still available to strrchr(), and it does the check
for us.

Note that this is just one implementation of find_last_dir_sep(). There
is an alternate implementation in compat/win32/path-utils.h. It doesn't
suffer from the same warning, as it does not use strrchr() and just
casts away const explicitly. That's not ideal, and eventually we may
want to conditionally teach it the same C23 generic trick that strrchr()
uses.  But it has been that way forever, and our goal here is just
quieting new warnings, not improving const-checking.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

The goal of this commit was to fix a const warning when compiling
with new versions of glibc, but ended up untangling a much deeper
problem.

The find_pseudo_merge() function does a bsearch() on the "commits"
pointer of a pseudo_merge_map. This pointer ultimately comes from memory
mapped from the on-disk bitmap file, and is thus not writable.

The "commits" array is correctly marked const, but the result from
bsearch() is returned directly as a non-const pseudo_merge_commit
struct. Since new versions of glibc annotate bsearch() in a way that
detects the implicit loss of const, the compiler now warns.

My first instinct was that we should be returning a const struct. That
requires apply_pseudo_merges_for_commit() to mark its local pointer as
const. But that doesn't work! If the offset field has the high-bit set,
we look it up in the extended table via nth_pseudo_merge_ext(). And that
function then feeds our const struct to read_pseudo_merge_commit_at(),
which writes into it by byte-swapping from the on-disk mmap.

But I think this points to a larger problem with find_pseudo_merge(). It
is not just that the return value is missing const, but it is missing
that byte-swapping! And we know that byte-swapping is needed here,
because the comparator we use for bsearch() also calls our
read_pseudo_merge_commit_at() helper.

So I think the interface is all wrong here. We should not be returning a
pointer to a struct which was cast from on-disk data. We should be
filling in a caller-provided struct using the bytes we found,
byte-swapping the values.

That of course raises the dual question: how did this ever work, and
does it work now? The answer to the first part is: this code does not
seem to be triggered in the test suite at all. If we insert a BUG("foo")
call into apply_pseudo_merges_for_commit(), it never triggers.

So I think there is something wrong or missing from the test setup, and
this bears further investigation. Sadly the answer to the second part
("does it work now") is still "no idea". I _think_ this takes us in a
positive direction, but my goal here is mainly to quiet the compiler
warning. Further bug-hunting on this experimental feature can be done
separately.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

The skip_prefix() function takes in a "const char *" string, and returns
via a "const char **" out-parameter that points somewhere in that
string. This is fine if you are operating on a const string, like:

  const char *in = ...;
  const char *out;
  if (skip_prefix(in, "foo", &out))
	...look at out...

It is also OK if "in" is not const but "out" is, as we add an implicit
const when we pass "in" to the function. But there's another case where
this is limiting. If we want both fields to be non-const, like:

  char *in = ...;
  char *out;
  if (skip_prefix(in, "foo", &out))
	*out = '\0';

it doesn't work. The compiler will complain about the type mismatch in
passing "&out" to a parameter which expects "const char **". So to make
this work, we have to do an explicit cast.

But such a cast is ugly, and also means that we run afoul of making this
mistake:

  const char *in = ...;
  char *out;
  if (skip_prefix(in, "foo", (const char **)&out))
	*out = '\0';

which causes us to write to the memory pointed by "in", which was const.

We can imagine these four cases as:

  (1) const in, const out
  (2) non-const in, const out
  (3) non-const in, non-const out
  (4) const in, non-const out

Cases (1) and (2) work now. We would like case (3) to work but it
doesn't. But we would like to catch case (4) as a compile error.

So ideally the rule is "the out-parameter must be at least as const as
the in-parameter". We can do this with some macro trickery. We wrap
skip_prefix() in a macro so that it has access to the real types of
in/out. And then we pass those parameters through another macro which:

  1. Fails if the "at least as const" rule is not filled.

  2. Casts to match the signature of the real skip_prefix().

There are a lot of ways to implement the "fails" part. You can use
__builtin_types_compatible_p() to check, and then either our
BUILD_ASSERT macros or _Static_assert to fail. But that requires some
conditional compilation based on compiler feature. That's probably OK
(the fallback would be to just cast without catching case 4). But we can
do better.

The macro I have here uses a ternary with a dead branch that tries to
assign "in" to "out", which should work everywhere and lets the compiler
catch the problem in the usual way. With an input like this:

  int foo(const char *x, const char **y);
  #define foo(in,out) foo((in), CONST_OUTPARAM((in), (out)))

  void ok_const(const char *x, const char **y)
  {
          foo(x, y);
  }

  void ok_nonconst(char *x, char **y)
  {
          foo(x, y);
  }

  void ok_add_const(char *x, const char **y)
  {
          foo(x, y);
  }

  void bad_drop_const(const char *x, char **y)
  {
          foo(x, y);
  }

gcc reports:

  foo.c: In function ‘bad_drop_const’:
  foo.c:2:35: error: assignment discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
      2 |     ((const char **)(0 ? ((*(out) = (in)),(out)) : (out)))
        |                                   ^
  foo.c:4:31: note: in expansion of macro ‘CONST_OUTPARAM’
      4 | #define foo(in,out) foo((in), CONST_OUTPARAM((in), (out)))
        |                               ^~~~~~~~~~~~~~
  foo.c:23:9: note: in expansion of macro ‘foo’
     23 |         foo(x, y);
        |         ^~~

It's a bit verbose, but I think makes it reasonably clear what's going
on. Using BUILD_ASSERT_OR_ZERO() ends up much worse. Using
_Static_assert you can be a bit more informative, but that's not
something we use at all yet in our code-base (it's an old gnu-ism later
standardized in C11).

Our generic macro only works for "const char **", which is something we
could improve by using typeof(in). But that introduces more portability
questions, and also some weird corner cases (e.g., around implicit void
conversion).

This patch just introduces the concept. We'll make use of it in future
patches.

Note that we rename skip_prefix() to skip_prefix_impl() here, to avoid
expanding the macro when defining the function. That's not strictly
necessary since we could just define the macro after defining the inline
function. But that would not be the case for a non-inline function (and
we will apply this technique to them later, and should be consistent).
It also gives us more freedom about where to define the macro. I did so
right above the definition here, which I think keeps the relevant bits
together.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

The "line" member of a packet_reader struct is marked as const. This
kind of makes sense, because it's not its own allocated buffer that
should be freed, and we often use const to indicate that. But it is
always writable, because it points into the non-const "buffer" member.

And we rely on this writability in places like send-pack and
receive-pack, where we parse incoming packet contents by writing NULs
over delimiters. This has traditionally worked because we implicitly
cast away the constness with strchr() like:

  const char *head;
  char *p;

  head = reader->line;
  p = strchr(head, ' ');

Since C23 libc provides a generic strchr() to detect this implicit
const removal, this now generate a compiler warning on some platforms
(like recent glibc).

We can fix it by marking "line" as non-const, as well as a few
intermediate variables (like "head" in the above example). Note that by
itself, switching to a non-const variable would cause problems with this
line in send-pack.c:

  if (!skip_prefix(reader->line, "unpack ", &reader->line))

But due to our skip_prefix() magic introduced in the previous commit,
this compiles fine (both the in and out-parameters are non-const, so we
know it is safe).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

This is another case where we implicitly drop the "const" from a pointer
by feeding it to strstr() and assigning the result to a non-const
pointer. This is OK in practice, since the const pointer originally
comes from a writable source (a strbuf), but C23 libc implementations
have started to complain about it.

We do write to the output pointer, so it needs to remain non-const. We
can just switch the input pointer to also be non-const in this case.  By
itself that would run into problems with calls to skip_prefix(), but
since that function has now been taught to match in/out constness
automatically, it just works without us doing anything further.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

In redact_sensitive_header(), a C23 implementation of libc will complain
that strstr() assigns the result from "const char *cookie" to "char
*semicolon".

Ultimately the memory is writable. We're fed a strbuf, generate a const
pointer "sensitive_header" within it using skip_iprefix(), and then
assign the result to "cookie".  So we can solve this by dropping the
const from "cookie" and "sensitive_header".

However, this runs afoul of skip_iprefix(), which wants a "const char
**" for its out-parameter. We can solve that by teaching skip_iprefix()
the same "make sure out is at least as const as in" magic that we
recently taught to skip_prefix().

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

In show_one_reflog_ent(), we're fed a writable strbuf buffer, which we
parse into the various reflog components. We write a NUL over email_end
to tie off one of the fields, and thus email_end must be non-const.

But with a C23 implementation of libc, strchr() will now complain when
assigning the result to a non-const pointer from a const one. So we can
fix this by making the source pointer non-const.

But there's a catch. We derive that source pointer by parsing the line
with parse_oid_hex_algop(), which requires a const pointer for its
out-parameter. We can work around that by teaching it to use our
CONST_OUTPARAM() trick, just like skip_prefix().

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

There's a typo in the comment, making it hard to understand. And the
macro itself is indented with spaces rather than tab.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Replace uses of `git config --list` (short or long) with the subcommand
`list` since `--list` is deprecated.

We will change the “man page” phrasing in gitcvs-migration(7) in the
next commit, since we are already visiting that sentence. But note
that we leave the “man page” phrasing in the sentence that we touch in
gittutorial(7) since it’s a tutorial and not a manual page. We can be
more wordy in a tutorial context.

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Let’s change the phrasing around the `linkgit` while we’re visiting this
file (see previous commit[1]).

We use the section syntax to refer to man pages, so writing “man page”
next to it is a bit redundant. We can be more concise and just lean on
the preposition “in”.

And in order to avoid this double “git”:

    see `git config list` in git-config(1) ...

We can rephrase to the subcommand, which is a typical pattern (config or
option followed by “in git-command(1)”).

† 1: Which also discusses why we do not change a similar phrasing
     in gittutorial(7)

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Add a test that verifies the 'git rev-list --maximal-only' option
produces the same set of commits as 'git merge-base --independent'. This
equivalence was noted when the feature was first created, but we are
about to update the implementation to use a common algorithm in this
case where the user intention is identical.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Add a performance test that compares 'git rev-list --maximal-only'
against 'git merge-base --independent'. These two commands are asking
essentially the same thing, but the rev-list implementation is more
generic and hence slower. These performance tests will demonstrate that
in the current state and also be used to show the equivalence in the
future.

We also add a case with '--since' to force the generic walk logic for
rev-list even when we make that future change to use the merge-base
algorithm on a simple walk.

When run on my copy of git.git, I see these results:

  Test                                      HEAD
  ----------------------------------------------
  6011.2: merge-base --independent          0.03
  6011.3: rev-list --maximal-only           0.06
  6011.4: rev-list --maximal-only --since   0.06

These numbers are low, but the --independent calculation is interesting
due to having a lot of local branches that are actually independent.

Running the same test on a fresh clone of the Linux kernel repository
shows a larger difference between the algorithms, especially because the
--independent algorithm is extremely fast when there are no independent
references selected:

  Test                                      HEAD
  ----------------------------------------------
  6011.2: merge-base --independent          0.00
  6011.3: rev-list --maximal-only           0.70
  6011.4: rev-list --maximal-only --since   0.70

Signed-off-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

The 'git rev-list --maximal-only' option filters the output to only
independent commits. A commit is independent if it is not reachable from
other listed commits. Currently this is implemented by doing a full
revision walk and marking parents with CHILD_VISITED to skip non-maximal
commits.

The 'git merge-base --independent' command computes the same result
using reduce_heads(), which uses the more efficient remove_redundant()
algorithm. This is significantly faster because it avoids walking the
entire commit graph.

Add a fast path in rev-list that detects when --maximal-only is the only
interesting option and all input commits are positive (no revision
ranges). In this case, use reduce_heads() directly instead of doing a
full revision walk.

In order to preserve the rest of the output filtering, this computation
is done opportunistically in a new prepare_maximal_independent() method
when possible. If successful, it populates revs->commits with the list
of independent commits and set revs->no_walk to prevent any other walk
from occurring. This allows us to have any custom output be handled
using the existing output code hidden inside
traverse_commit_list_filtered(). A new test is added to demonstrate that
this output is preserved.

The fast path is only used when no other flags complicate the walk or
output format: no UNINTERESTING commits, no limiting options (max-count,
age filters, path filters, grep filters), no output formatting beyond
plain OIDs, and no object listing flags.

Running the p6011 performance test for my copy of git.git, I see the
following improvement with this change:

  Test                                     HEAD~1  HEAD
  ------------------------------------------------------------
  6011.2: merge-base --independent          0.03   0.03 +0.0%
  6011.3: rev-list --maximal-only           0.06   0.03 -50.0%
  6011.4: rev-list --maximal-only --since   0.06   0.06 +0.0%

And for a fresh clone of the Linux kernel repository, I see:

  Test                                     HEAD~1  HEAD
  ------------------------------------------------------------
  6011.2: merge-base --independent          0.00   0.00 =
  6011.3: rev-list --maximal-only           0.70   0.00 -100.0%
  6011.4: rev-list --maximal-only --since   0.70   0.70 +0.0%

In both cases, the performance is indeed matching the behavior of 'git
merge-base --independent', as expected.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Further work to adjust the codebase for C23 that changes functions
like strchr() that discarded constness when they return a pointer into
a const string to preserve constness.

* jk/c23-const-preserving-fixes-more:
  git-compat-util: fix CONST_OUTPARAM typo and indentation
  refs/files-backend: drop const to fix strchr() warning
  http: drop const to fix strstr() warning
  range-diff: drop const to fix strstr() warnings
  pkt-line: make packet_reader.line non-const
  skip_prefix(): check const match between in and out params
  pseudo-merge: fix disk reads from find_pseudo_merge()
  find_last_dir_sep(): convert inline function to macro
  run-command: explicitly cast away constness when assigning to void
  pager: explicitly cast away strchr() constness
  transport-helper: drop const to fix strchr() warnings
  http: add const to fix strchr() warnings
  convert: add const to fix strchr() warnings

"git config list" is the official way to spell "git config -l" and
"git config --list".  Use it to update the documentation.

* kh/doc-config-list:
  doc: gitcvs-migration: rephrase “man page”
  doc: replace git config --list/-l with `list`

"git rev-list --maximal-only" has been optimized by borrowing the
logic used by "git show-branch --independent", which computes the
same kind of information much more efficiently.

* ds/rev-list-maximal-only-optim:
  rev-list: use reduce_heads() for --maximal-only
  p6011: add perf test for rev-list --maximal-only
  t6600: test --maximal-only and --independent

Signed-off-by: Junio C Hamano <gitster@pobox.com>